Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(logs): messages infinite scroll and live refresh #2214

Merged
merged 16 commits into from
May 29, 2024

Conversation

bodinsamuel
Copy link
Collaborator

Context

Fixes NAN-998
Contributes to NAN-908

More or less the same than #2213 The main diff here is that we don't need to update the current page state, so we use two cursor cursorBefore and cursorAfter the former to live refresh (prepend) and the later to do infinite scroll (append).

Changes

  • Add cursorBefore and cursorAfter to searchMessages
    Note: there is no concept of search_before in Elasticsearch, you just reverse the sorting order and re-reverse the results to achieve it.

  • Implement live refresh and infinite scroll in the UI

Test

**(note the scrollbar changing position because we prepend, it's very hard to manage unless we use our own scroll bar

Screen.Recording.2024-05-28.at.10.42.08.mov

@bodinsamuel bodinsamuel self-assigned this May 28, 2024
Copy link

linear bot commented May 28, 2024

Base automatically changed from feat/logs-pagination to master May 29, 2024 12:00
@bodinsamuel bodinsamuel marked this pull request as ready for review May 29, 2024 12:09
@@ -66,7 +66,7 @@ export const ShowMessage: React.FC<{ message: MessageRow }> = ({ message }) => {
return { code: { padding: '0', whiteSpace: 'pre-wrap' } };
}}
>
{JSON.stringify({ error: message.error || undefined, output: message.meta || undefined }, null, 2)}
{JSON.stringify({ error: message.error?.message || undefined, output: message.meta || undefined }, null, 2)}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the || undefined necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes because messge.error can be null and null is not automatically removed from json stringify, but undefined will be stripped.

return hit._source!;
});
if (opts.cursorBefore) {
// In case we set before we have to reverse the message since we inverted the sort
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a link to elastic/elasticsearch#29449 Might be handy if we ever run into the limitation mentioned in the issue

@bodinsamuel bodinsamuel enabled auto-merge (squash) May 29, 2024 13:44
@bodinsamuel bodinsamuel merged commit c2f0490 into master May 29, 2024
20 of 21 checks passed
@bodinsamuel bodinsamuel deleted the feat/logs-message-pagination branch May 29, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants